feat(agglayer): RBAC-based access control for bridge roles by Fumuran · Pull Request #3130 · 0xMiden/protocol

Fumuran · 2026-06-24T19:30:17Z

Summary

Refactor phase of #2706. Replaces the AggLayer bridge's hard-coded admin / GER-injector / GER-remover account-ID storage slots with the miden-standards access-control stack (Ownable2Step + RoleBasedAccessControl + Authority).

Per the discussion on the issue, this uses RBAC rather than Ownable2Step alone, with a separate governance owner over three subordinate operational roles. Each role-gated bridge procedure now calls authority::assert_authorized, which checks the note sender holds the role mapped to that procedure.

Roles

Governance owner (Ownable2Step): top-level authority; grants/revokes roles, two-step transferable.
FAUCET_ADMIN → register_faucet, store_faucet_metadata_hash
GER_INJECTOR → update_ger
GER_REMOVER → remove_ger

Changes

miden-standards: add RoleAssignment + RoleBasedAccessControl::with_roles(...) to seed initial role members at construction; AccessControl::Rbac gains a members field (yields a seeded RBAC instead of empty()).
bridge MASM: remove the three account-ID slots and bespoke assert_sender_is_* procs; gate the four privileged procedures via authority::assert_authorized.
bridge Rust: AggLayerBridge is now stateless; add BridgeRoleMember, role-symbol + procedure-root accessors, and the fixed procedure→role map. create_bridge_account now takes (seed, owner, Vec<BridgeRoleMember>).
build.rs: include the RBAC stack when computing BRIDGE_CODE_COMMITMENT.
Update SPEC.md, agglayer integration tests, and bench setups. Add a with_roles-equals-runtime-grant_role equivalence test and a procedure→role map pinning test.

Out of scope (follow-up)

On-chain role-management notes (grant_role / revoke_role / transfer_ownership / accept_ownership) plus their allowlist entries and Rust builders. Roles are seeded and enforced, but rotation is not yet exercisable on-chain — hence this is the refactor phase (Refs #2706, not Closes).

Testing

cargo +nightly fmt --all --check and cargo clippy (affected crates, --all-targets): clean.
Tests pass: agglayer suite (69), rbac/authority/pausable script tests (42), miden-standards (119).

🤖 Generated with Claude Code

Replace the bridge's hard-coded admin / GER-injector / GER-remover account-ID storage slots with the miden-standards access-control stack (Ownable2Step + RoleBasedAccessControl + Authority). The role-gated procedures (register_faucet, store_faucet_metadata_hash, update_ger, remove_ger) now call authority::assert_authorized, which checks the note sender holds the FAUCET_ADMIN / GER_INJECTOR / GER_REMOVER role mapped to that procedure. A governance owner can grant/revoke roles and transfer ownership via Ownable2Step. - miden-standards: add RoleAssignment + RoleBasedAccessControl::with_roles to seed initial role members at construction; AccessControl::Rbac gains a `members` field. - bridge MASM: drop the three ID slots / bespoke assert_sender_is_* procs; gate the four privileged procedures via authority::assert_authorized. - bridge Rust: add BridgeRoleMember, role-symbol + procedure-root accessors, and the fixed procedure->role map; create_bridge_account now takes an owner and a Vec<BridgeRoleMember> instead of three bare account IDs. - build.rs: include the RBAC stack when computing BRIDGE_CODE_COMMITMENT. - update SPEC.md, agglayer integration tests, and bench setups. On-chain role-management notes (grant/revoke role, transfer ownership) are a planned follow-up, so role rotation is not yet exercisable on-chain. Refs #2706. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

AggLayerBridge is a zero-field component; use the bare value via its Default/From impl (matching PausableManager and BurnAllowAll) instead of an empty new(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Represent initial role membership as BTreeMap<RoleSymbol, BTreeSet<AccountId>> instead of Vec<RoleAssignment>. Duplicate roles and duplicate members are now impossible by construction, so with_roles drops the two dedup passes (and the DuplicateRole / DuplicateMember error variants), and the RoleAssignment type is removed. The bridge's role grouping collapses to a single entry()-based loop. No behavior change: seeding still equals runtime grant_role (equivalence test passes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

- bridge: resolve procedure roots via the procedure_root! macro + a component code() getter, matching the PausableManager pattern (removes the ad-hoc bridge_procedure_root helper). - bridge: BridgeRoleMember variants now carry a Vec<AccountId> so all holders of a role are grouped in one entry; rbac_role_members collects via entry()+extend. - docs: link role names to their BridgeRoleMember variants; tighten the bridge component and builder doc comments; bullet-list the with_roles Errors section. - masm: drop the redundant storage-slots comment. - SPEC: switch stored-ID wording to role-membership wording and keep a single authority::assert_authorized explanation (in Administration) instead of repeating it in every note/procedure section. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

- bridge: move the component namespace and the component-code getter into AggLayerBridge (AggLayerBridge::COMPONENT_NAMESPACE and AggLayerBridge::code()), matching the PausableManager shape. - rbac: trim the seeded roles field doc to the essential summary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

mmagician

Overall LGTM!

Resolve import conflicts in lib.rs and the agglayer test files, and adapt to next's AccountId::dummy 4-arg signature (AssetCallbackFlag) in the bridge test helpers. The bridge-account test call sites keep using the create_existing_bridge_account_with_roles helper, so next's direct create_existing_bridge_account import is dropped where unused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

partylikeits1983

Looks really good, I like the simplification that the RBAC provides.

There are a few minor things I think should be addressed before merging.

partylikeits1983 · 2026-07-06T10:45:20Z

+                RoleBasedAccessControl::with_roles(members)
+                    .expect("initial RBAC role assignments should be valid")
+                    .into(),


Nit: This will panic instead of returning an error if the user passes an empty member set (or a set with more than u32::MAX members).

Can we return an error here instead of panicking? And add tests for both error cases?

Indeed, I overlooked it. I'll update it to return an error and add tests for it

Hmm, this update will complicate things a bit, since we expect here to have an iterator of components: changing this into iterator of results will require some changes in quite a lot of places, so I'm not sure that it's forth it. For now I'll just move this checks to the Into<AccountComponent> implementation, but in general I agree that we should somehow return an error here instead of panic.

- bridge: replace the BridgeRoleMember enum with a BridgeRoles struct (BTreeSet per role) and a fallible BridgeRoles::new that rejects an empty holder set for any role (new AgglayerBridgeError::EmptyBridgeRole), so a bridge can't be deployed with an unfillable role. create_bridge_account takes BridgeRoles. - standards: make RoleBasedAccessControl::with_roles infallible (drop empty member sets instead of erroring/panicking; remove RbacError and the IntoIterator expect). Add reusable role_config_key / role_membership_key helpers and use them in the From impl instead of building raw words. - testing: move the shared create_existing_bridge_account_with_roles / bridge_test_owner helper into miden_agglayer::testing so the test crate and bench share one copy. - tests: add ERR_SENDER_LACKS_ROLE tests for update_ger (GER_INJECTOR) and register_faucet via CONFIG (FAUCET_ADMIN), and a BridgeRoles::new empty-role test. - SPEC: point the role-management follow-up TODO at #3046; fix a doc link. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

- changelog: correct the [BREAKING] entry to reflect the BridgeRoles signature (create_bridge_account takes (seed, owner, BridgeRoles)) instead of the removed BridgeRoleMember type. - rbac: note that the u32 member-count conversion in the From impl is purely defensive / infallible. - test: add test_rbac_with_roles_drops_empty_role locking in the empty-role drop semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

create_rbac_account_with_members(owner, BTreeMap::new()) covers the empty-members case, so the separate owner-only helper is unnecessary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

It was only used by the shared create_existing_bridge_account_with_roles test helper, so fold the build_existing() call into that helper (via the crate-private create_bridge_account_builder) and drop the redundant public wrapper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

- bridge: drop the verbose BridgeRoles struct doc paragraph (the empty-role rejection is already documented on BridgeRoles::new). - changelog: trim the breaking-change entry per review suggestion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Reconcile the RBAC access-control refactor with next's new DEREGISTER_AGG_FAUCET feature: - Convert deregister_faucet from assert_sender_is_bridge_admin (removed by this branch) to authority::assert_authorized, mapped to the FAUCET_ADMIN role (procedure_roles + a new procedure_root!/deregister_faucet_root accessor). - Update next's deregister tests to the create_existing_bridge_account_with_roles helper and ERR_SENDER_LACKS_ROLE; update the procedure-roles mapping test to five entries. - SPEC: keep the RBAC role wording with next's shifted section numbers and align the DEREGISTER_AGG_FAUCET section to the FAUCET_ADMIN role terminology. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Post-merge review nits: the config/deregister note-script headers, the register_faucet / store_faucet_metadata_hash panic docs, and the deregister note builder still referred to the 'bridge admin'; update them to the FAUCET_ADMIN role that authority::assert_authorized now enforces. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Fumuran · 2026-07-07T13:15:17Z

Current state looks fine for me, but I would wait for the #3215 to be merged first, then I'll update this PR and only then re-request the reviews.

Reconciles the AggLayer bridge RBAC work with #3215, which made role administration fully role-based and removed the Ownable2Step super-admin over the role graph. - Add RoleBasedAccessControl::with_roles(admins, roles) to miden-standards: seeds the built-in ADMIN role plus arbitrary operator roles at genesis (empty member sets are dropped). This lets the bridge stay born-functional while #3215's AccessControl::Rbac convenience enum only seeds ADMIN. - Rework the bridge onto the ADMIN-based stack (RoleBasedAccessControl + Authority, no Ownable2Step): create_bridge_account now takes (seed, admin, BridgeRoles); admin seeds ADMIN, which administers the operational FAUCET_ADMIN / GER_INJECTOR / GER_REMOVER roles. - Update build.rs code-commitment composition, tests/bench, SPEC, and CHANGELOG for the new signature and administration model.

Addresses two review comments: 1. Rename the non-admin faucet role FAUCET_ADMIN -> FAUCET_MNGR (symbol) / faucet_manager (Rust identifiers). 'Admin' was misleading for an operational role administered by ADMIN; an admin variation would have read as FAUCET_ADMIN_ADMIN. Also renames the outdated bridge_admin test wallets/comments to faucet_manager (they hold the faucet role, not ADMIN). 2. Disambiguate the two RBAC maps that were both spelled 'roles': - role-members map: RoleBasedAccessControl::with_roles -> with_role_members (field initial_roles -> initial_role_members). - procedure-roles map: the 'roles' field of Authority::RbacControlled and AccessControl::Rbac -> procedure_roles.

Fumuran · 2026-07-09T13:57:22Z

One thing worth mentioning: I renamed FAUCET_ADMIN role to the FAUCET_MNGR because it could be confusing: this role is not an admin role for some other one. After that I realized that we have a problem here.

Currently we create our role names internally using ShortCapitalString which allows us to have at most 12 chars in the name. The problem is that any attempt to create an admin role for any of our roles (for example, create the GER_INJECTOR_ADMIN) will result in an error: name will be too long. So we should either change the way we construct the role name, or update the ShortCapitalString making its MAX_LENGTH a constant parameter:

pub(crate) struct ShortCapitalString<const MAX_LEN: u8>(String);

mmagician

LGTM, the remaining comments are all minor and should be trivial to resolve - good to merge after that ✅

onurinanc

LGTM! A few non-blocking nits. Only reviewed refactoring parts though.

bobbinth · 2026-07-12T06:48:07Z

@Fumuran - there are now some small merge conflicts - could you resolve them when you get a chance?

…er-rbac-roles # Conflicts: # crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm # crates/miden-testing/tests/scripts/rbac.rs

claude added 4 commits June 24, 2026 22:19

docs: add changelog entry for bridge RBAC access control (#3130)

4e43d7d

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

refactor(agglayer): drop redundant AggLayerBridge::new()

2dfe41a

AggLayerBridge is a zero-field component; use the bare value via its Default/From impl (matching PausableManager and BurnAllowAll) instead of an empty new(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Fumuran commented Jul 1, 2026

View reviewed changes

Fumuran and others added 2 commits July 1, 2026 21:32

Merge branch 'next' into fumuran-claude/agglayer-rbac-roles

e4dfa28

Fumuran commented Jul 1, 2026

View reviewed changes

Comment thread crates/miden-agglayer/src/bridge.rs Outdated

Comment thread crates/miden-agglayer/src/bridge.rs Outdated

Comment thread crates/miden-standards/src/account/access/rbac.rs Outdated

Fumuran marked this pull request as ready for review July 1, 2026 19:54

Fumuran added the pr-from-maintainers PRs that come from internal contributors or integration partners. They should be given priority label Jul 1, 2026

Fumuran requested review from mmagician and partylikeits1983 July 1, 2026 19:54

mmagician added the agglayer PRs or issues related to AggLayer bridging integration label Jul 1, 2026

mmagician reviewed Jul 1, 2026

View reviewed changes

partylikeits1983 reviewed Jul 6, 2026

View reviewed changes

Comment thread crates/miden-agglayer/src/bridge.rs

Comment thread crates/miden-testing/tests/agglayer/update_ger.rs

Comment thread bin/bench-transaction/src/context_setups.rs Outdated

onurinanc mentioned this pull request Jul 6, 2026

L-15: RBAC Owner Has Unconditional Super-Admin Authority Over All Roles #3214

Closed

partylikeits1983 reviewed Jul 6, 2026

View reviewed changes

onurinanc mentioned this pull request Jul 6, 2026

fix(standards): make RBAC role administration fully role-based #3215

Merged

claude added 2 commits July 6, 2026 21:03

Fumuran mentioned this pull request Jul 7, 2026

Align Agglayer role names with their procedures #3228

Open

claude added 2 commits July 7, 2026 14:40

test: drop redundant create_rbac_account_with_owner helper

8c004b4

create_rbac_account_with_members(owner, BTreeMap::new()) covers the empty-members case, so the separate owner-only helper is unnecessary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Fumuran commented Jul 7, 2026

View reviewed changes

Comment thread crates/miden-agglayer/src/bridge.rs Outdated

Comment thread crates/miden-agglayer/src/bridge.rs Outdated

Comment thread CHANGELOG.md Outdated

claude added 3 commits July 7, 2026 15:08

claude and others added 3 commits July 9, 2026 13:10

chore: update docs, reorganize test folder (no code changes)

3a3ba8f

Fumuran requested review from mmagician and partylikeits1983 July 9, 2026 13:57

mmagician reviewed Jul 9, 2026

View reviewed changes

Comment thread crates/miden-agglayer/src/bridge.rs

Comment thread crates/miden-agglayer/SPEC.md

Comment thread crates/miden-standards/src/account/access/rbac.rs Outdated

Comment thread crates/miden-testing/tests/scripts/rbac.rs

mmagician reviewed Jul 9, 2026

View reviewed changes

Comment thread crates/miden-standards/src/account/access/rbac.rs

mmagician requested a review from onurinanc July 9, 2026 15:40

mmagician approved these changes Jul 9, 2026

View reviewed changes

Comment thread crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Outdated

Comment thread crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Outdated

Comment thread crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Outdated

onurinanc approved these changes Jul 10, 2026

View reviewed changes

Comment thread crates/miden-standards/src/account/access/rbac.rs Outdated

Comment thread crates/miden-standards/src/account/access/rbac.rs Outdated

Fumuran mentioned this pull request Jul 13, 2026

Increase length of the RBAC role names #3287

Open

claude and others added 4 commits July 15, 2026 17:45

Merge remote-tracking branch 'origin/next' into fumuran-claude/agglay…

223a2a0

…er-rbac-roles # Conflicts: # crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm # crates/miden-testing/tests/scripts/rbac.rs

refactor: update doc comments, optimize RBAC constructors

95c4e04

chore: update doc comment

a51a672

Merge branch 'next' into fumuran-claude/agglayer-rbac-roles

5343b63

Fumuran added this pull request to the merge queue Jul 15, 2026

Merged via the queue into next with commit 2295105 Jul 15, 2026
19 checks passed

Fumuran deleted the fumuran-claude/agglayer-rbac-roles branch July 15, 2026 18:44

Uh oh!

Conversation

Fumuran commented Jun 24, 2026

Summary

Roles

Changes

Out of scope (follow-up)

Testing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mmagician left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

partylikeits1983 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

partylikeits1983 Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Fumuran Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Fumuran Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fumuran commented Jul 7, 2026

Uh oh!

Fumuran commented Jul 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mmagician left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

onurinanc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bobbinth commented Jul 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

onurinanc left a comment •

edited

Loading